Fix Entra ID tenant parsing for multi-segment STSURL authorities by cheenamalhotra · Pull Request #4521 · dotnet/SqlClient

cheenamalhotra · 2026-08-08T08:12:20Z

Description

Connecting to the Dataverse / Dynamics 365 TDS endpoint with Authentication=Active Directory Service Principal fails on 7.0.0+ with ClientSecretCredential authentication failed.

Dataverse returns an OAuth v1 style STSURL in the FEDAUTHINFO TDS token:

https://login.microsoftonline.com/{tenantId}/oauth2/authorize

ActiveDirectoryAuthenticationProvider.AcquireTokenAsync split the authority at the last /, so ClientSecretCredential received the literal string "authorize" as its tenant id, and AuthorityHost became https://login.microsoftonline.com/{tenantId}/oauth2/. Azure SQL and Fabric return the bare https://login.microsoftonline.com/{tenantId} form, so only multi-segment STSURLs were affected. The same split existed in 6.x, where the older Azure.Identity / MSAL combination happened to tolerate the malformed authority.

Approach

The tenant is now taken from the first path segment of the authority URL rather than the last, so trailing endpoint suffixes (/oauth2/authorize, /oauth2/v2.0/token, ...) are ignored:

New TryParseAuthority helper splits the STSURL into an authority host (https://login.microsoftonline.com/), a tenant, and a normalized MSAL authority (host + tenant).
The normalized authority is now what gets passed to MSAL's PublicClientAppKey / WithAuthority, so the interactive, password, integrated, and device-code flows are fixed too, not just the Azure.Identity based ones.
The old last-separator split has been removed rather than kept as a fallback. Entra ID authorities are always absolute HTTPS URLs, and both MSAL's WithAuthority and Azure.Identity's AuthorityHost (new Uri(...)) require an absolute URI, so the fallback could never have produced a working credential. An unparseable authority now raises a clear AuthenticationException naming the offending value and the expected shape instead of failing obscurely deeper in the stack.
Added a catch (AuthenticationException) { throw; } guard in AcquireTokenAsync so provider-raised authentication errors are no longer re-wrapped by the generic catch-all as "Unexpected error". This also improves the pre-existing "authentication method not supported" path.

Backwards compatibility

No public API changes. Behavior is unchanged for the bare https://login.microsoftonline.com/{tenantId} authority that Azure SQL, Fabric, and Synapse send. The only behavior difference for existing working scenarios is the improved error message when an authority is malformed.

Issues

Fixes #4496

Testing

Added AuthorityParsingTests in src/Microsoft.Data.SqlClient.Extensions/Azure/test/, covering only authority shapes that Entra ID actually documents:

Bare tenant authority (Azure SQL / Fabric), with and without a trailing slash
v1.0 /oauth2/authorize endpoint (the Dataverse repro)
v2.0 /oauth2/v2.0/token endpoint
National clouds: login.microsoftonline.us and login.partner.microsoftonline.cn
Domain-name tenant and the common / organizations placeholders
Rejection cases: an authority with no tenant segment, and an empty STSURL

Results: 12/12 new tests pass; 34 passed / 1 skipped across the non-integration Azure test suite on net9.0. The remaining AADConnectionTest failure in the full run is a pre-existing integration test that requires a live Azure SQL server and network access.

Manual verification against a real Dataverse TDS endpoint has not been performed and would be valuable before merge, since that environment is not available in this workspace.

Guidelines

Please review the contribution guidelines before submitting a pull request:

Fixes dotnet#4496 The Dataverse/Dynamics 365 TDS endpoint returns an ADAL v1 style STSURL ("https://login.microsoftonline.com/{tenantId}/oauth2/authorize") in the FEDAUTHINFO token. AcquireTokenAsync split the authority at the last '/', so the tenant was parsed as the literal "authorize" and the authority host became ".../oauth2/", causing authentication to fail. The tenant is now taken from the first path segment of the authority URL, ignoring trailing endpoint suffixes, and the normalized authority (host + tenant) is used for the MSAL public client application. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4e906c9-daf3-46f8-83e7-ef805d81fdfb

Entra ID authorities (and therefore the STSURL in FEDAUTHINFO) are always absolute HTTPS URLs, and both MSAL's WithAuthority and Azure.Identity's AuthorityHost require an absolute URI, so the legacy last-separator split could never produce a working credential for anything else. Replace the fallback with TryParseAuthority, which rejects such authorities up front with a clear AuthenticationException instead of failing obscurely later. Also stop re-wrapping AuthenticationException in the generic catch block. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4e906c9-daf3-46f8-83e7-ef805d81fdfb

Copilot

Pull request overview

This PR fixes Entra ID authority parsing when SQL Server (notably Dataverse/Dynamics 365 TDS) returns multi-segment STSURL values (e.g., /oauth2/authorize) in the FEDAUTHINFO token, ensuring the tenant is parsed correctly and the MSAL authority is normalized.

Changes:

Add TryParseAuthority to split an absolute HTTPS STSURL into authorityHost, tenant, and a normalized msalAuthority (host + tenant).
Use normalized msalAuthority for MSAL-based flows (via PublicClientAppKey / WithAuthority) and fail fast with a clearer AuthenticationException when the STSURL is malformed.
Add unit tests covering documented authority URL shapes, including v1/v2 endpoints and national cloud hosts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs	Normalizes STSURL parsing (first path segment tenant) and ensures MSAL/Azure.Identity receive correct authority/tenant; improves exception pass-through.
src/Microsoft.Data.SqlClient.Extensions/Azure/test/AuthorityParsingTests.cs	Adds test coverage for supported STSURL shapes and rejection cases for missing tenant/empty authority.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

edwardneal · 2026-08-08T15:08:24Z

The test below connects to a Power Apps Developer Dataverse instance; this test fails on main and passes with this PR.

[Fact]
public void ActiveDirectoryDefaultAuthenticationToDataverse_Succeeds()
{
    const string ConnectionString = "Data Source=<org name>.crm11.dynamics.com;User ID=<user id>;Encrypt=True;TrustServerCertificate=True;Authentication=ActiveDirectoryDefault;";

    using SqlConnection conn = new(ConnectionString);
    conn.Open();

    using SqlCommand cmd = new("select * from sys.databases", conn);
    using SqlDataReader rd = cmd.ExecuteReader();

    while (rd.Read())
    { }
}

Address review feedback: - The 'audience' local no longer held the last path segment after the parsing fix; it holds the tenant that is passed to Azure.Identity as TenantId. Rename the locals and the TokenCredentialKey fields to authorityHost/tenant so the names match what they carry, and refresh the surrounding comment accordingly. - Add a 'consumers' placeholder case to AuthorityParsingTests, which the TryParseAuthority documentation already calls out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4e906c9-daf3-46f8-83e7-ef805d81fdfb

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs:268

Typo in comment: "DefaultAzureCredenial" should be "DefaultAzureCredential" to match the type name and avoid confusion during future maintenance/searching.

                // Cache DefaultAzureCredenial based on scope, authority host, tenant, and clientId

cheenamalhotra and others added 2 commits August 8, 2026 01:05

Copilot AI lite review requested due to automatic review settings August 8, 2026 08:12

cheenamalhotra requested a review from a team as a code owner August 8, 2026 08:12

github-project-automation Bot added this to SqlClient Board Aug 8, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Aug 8, 2026

Copilot started reviewing on behalf of cheenamalhotra August 8, 2026 08:12 View session

cheenamalhotra added this to the 7.1.0-preview3 milestone Aug 8, 2026

cheenamalhotra added the Hotfix 7.0.3 PRs targeting main that should be backported to release/7.0 branch for next release. label Aug 8, 2026

cheenamalhotra assigned benrr101 and apoorvdeshmukh Aug 8, 2026

Copilot AI reviewed Aug 8, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs

Comment thread src/Microsoft.Data.SqlClient.Extensions/Azure/test/AuthorityParsingTests.cs

Copilot AI review requested due to automatic review settings August 10, 2026 17:36

Copilot started reviewing on behalf of cheenamalhotra August 10, 2026 17:37 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

apoorvdeshmukh approved these changes Aug 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix Entra ID tenant parsing for multi-segment STSURL authorities - #4521

Fix Entra ID tenant parsing for multi-segment STSURL authorities#4521
cheenamalhotra wants to merge 3 commits into
dotnet:mainfrom
cheenamalhotra:dev/cheena/ideal-tribble

cheenamalhotra commented Aug 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

edwardneal commented Aug 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

cheenamalhotra commented Aug 8, 2026

Description

Approach

Backwards compatibility

Issues

Testing

Guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

edwardneal commented Aug 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants